Fix various solar position calculations when DatetimeIndex.unit is not ns
#1948
Conversation
|
Ready for review. The codecov decrease can be ignored, I think. It is because coverage is based on the python 3.7 job, which is too old to support the pandas version needed to exercise the tested behavior. That means the new tests are not being run on the 3.7 job, and so their lines are counting against coverage. However, you can see that the jobs with later python versions are indeed running them (the skips are for Additionally: we probably don't want to include it as a formal test due to the polars dependency, but I'll note that this PR also passes the reproducer code in #1932 (comment). |
| 2 * times.view(np.int64) - times.normalize().view(np.int64) - | ||
| naive_times.view(np.int64)) | ||
| if times.tz is None: | ||
| times = times.tz_localize('utc') |
There was a problem hiding this comment.
Should we warn about this localization? Maybe just a note. The function promises hour angle in the time zone for the longitude.
There was a problem hiding this comment.
In case it's not obvious, this localization retains the existing behavior of assuming naive inputs to be in UTC; the rewrite for the unit issue just happens to make it more explicit. See for v0.9.5:
In [1]: import pvlib # v0.9.5
...: import pandas as pd
...: for tz in [None, 'utc', 'Etc/GMT+5']:
...: times = pd.date_range('2019-06-01 12:00', freq='h', periods=2, tz=tz)
...: eot = pvlib.solarposition.equation_of_time_spencer71(times.dayofyear)
...: ha = pvlib.solarposition.hour_angle(times, -80, eot)
...: print(f'hour angles, {tz=}:', ha)
...:
hour angles, tz=None: [-79.36192279 -64.36192279]
hour angles, tz='utc': [-79.36192279 -64.36192279]
hour angles, tz='Etc/GMT+5': [-4.36192279 10.63807721]The docstring says that times must be localized (Corresponding timestamps, must be localized to the timezone for the longitude.), so I guess naive times are technically not supported. Other solar position functions assume UTC for naive inputs, and don't warn, so I think I'm inclined to keep that behavior here as well.
docs/sphinx/source/referencefor API changes.docs/sphinx/source/whatsnewfor all changes. Includes link to the GitHub Issue with:issue:`num`or this Pull Request with:pull:`num`. Includes contributor name and/or GitHub username (link with:ghuser:`user`).remote-data) and Milestone are assigned to the Pull Request and linked Issue.